• ❌ - login: @darklight3it / name: Davide Melfi . The commit (afdf3b5, 849ecf3, d3e1646, e42bbdc, f4c0e4f) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

Codecov Report

❌ Patch coverage is 99.08884% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.4%. Comparing base (cef3317) to head (e42bbdc).

@@           Coverage Diff           @@
##            main   #3489     +/-   ##
=======================================
+ Coverage   83.7%   84.4%   +0.7%     
=======================================
  Files        126     126             
  Lines      25386   25825    +439     
=======================================
+ Hits       21255   21818    +563     
+ Misses      4131    4007    -124     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@darklight3it - Can you clarify the intended use case for constructing ResourceMetrics and calling PushMetricExporter::export() directly?

In the normal metrics pipeline, instrumentation libraries create measurements through instruments, and the SDK/MetricReader produces ResourceMetrics for the exporter. So the example shared in the desc looks like a lower-level/exporter use case rather than normal instrumentation.

Is the target use case pre-aggregated metrics from an external source, or directly sending measurements to an exporter while bypassing SDK aggregation? If it is the latter, I am not sure ResourceMetrics constructors are the right API shape, since these types represent already-aggregated metric points, not raw measurements. The spec direction for third-party/pre-processed metric sources seems closer to a MetricProducer plugged into a MetricReader.

Yes, the target use case is pre-aggregated metrics from an external source. The aggregation is already done, the Rust side only needs to serialize and transport the resulting ResourceMetrics via OTLP.

The challenge I'm running into is that MetricExporter::export() is public and intended to be callable externally, but its argument (ResourceMetrics) can't be constructed outside the crate.

This makes it difficult to use the exporter trait independently of the SDK's internal reader pipeline. I think opening up ResourceMetrics would better support the modularity that motivated having opentelemetry-otlp as a separate crate and would bring metrics in line with spans and logs, where SpanData and LogRecord already have public fields and work well for custom exporter implementations.

Thanks, that use case makes sense.

I think this is closer to the spec’s MetricProducer path, though. MetricProducer is meant for third-party/pre processed metric sources, and it plugs into a MetricReader. So the shape would be more like: external aggregated source -> MetricProducer -> MetricReader -> exporter.

That makes me hesitant to expose constructors for the full ResourceMetrics tree and encourage calling PushMetricExporter::export() directly. It feels like we would be making a lot of SDK metric data internals part of the stable public construction API. I’d prefer we discuss a smaller API for this use case, maybe MetricProducer-style, instead of opening up all these data types as-is.

Thanks @lalitb for the thoughtful feedback, I appreciate the discussion.

On MetricProducer:

• It's not currently available Removed in Remove MetricProducer #2105 for 1.0 scope (Metrics beta/rc/stable release timelines #1719), planned post-stable but not yet reintroduced ([Feature]: Implement MetricProducer #2505, draft revert: reintroduce external producers #3025).
• It's a pull model, my use case is push. My metrics arrive already aggregated and need to be exported immediately. There's no Rust-side MetricReader — adding one would introduce buffering and timer latency for what is a direct pass-through.

On the concern about opening up the full ResourceMetrics tree:

• The builder doesn't expose fields directly. It keeps internal data behind public methods, the same pattern as the existing getters (and in other types throughout the repo). The stability surface is symmetric with what's already committed: export() is public, the getters are public, ResourceMetrics::Default() already allows external construction. The builder just lets you construct a populated instance.

• The PR does touch nested types (ScopeMetrics, Metric, etc.) with the same builder pattern. But the same argument applies uniformly: each of these types already has public getters that expose their shape. The builders don't add new surface area, they provide the construction counterpart to what's already readable. New fields and new subtypes should already be maintained like this for the reading part.

• If maintenance is a concern, a derive macro (like typed-builder) could auto-generate the builders from the struct definitions, keeping them in sync automatically. I considered that approach but chose hand-written builders to avoid adding a proc-macro dependency to the crate, which I think is the right choice for a foundational library like this.

I think MetricProducer is still the closer conceptual fit - and it would be better to invest in bringing this feature back.

The “pull” part there is only inside the SDK: the MetricReader asks the producer for already-aggregated data. It does not mean the final export has to be pull-based. The final exporter can still be a push exporter sending OTLP.

So the shape would be: external pre-aggregated source -> MetricProducer -> MetricReader -> PushMetricExporter.

For the immediate-export case, we may need an API that lets the reader collect/export on demand, but I do not think that means PushMetricExporter::export should become the public ingestion path or that we should expose builders for the whole ResourceMetrics tree.

I understand the preference for an internal path, but if I'm reading the proposed workflow correctly, the changes in this PR are still necessary for that to work.

// simplified MetricProducer shape to illustrate the point
struct MyProducer { buffer: Mutex<Vec<ScopeMetrics>> }

impl MetricProducer for MyProducer {
    fn produce(&self) -> Vec<ScopeMetrics> {
         self.buffer.lock().unwrap().drain(..).collect()
     }
 }

fn receive_external_metrics(data: ExternalData) {
     let scope_metrics: Vec<ScopeMetrics> = data.into(); // needs constructors for
                                                          // ScopeMetrics, Metric,
                                                          // Sum/Gauge/Histogram,
                                                          // DataPoint
     PRODUCER.buffer.lock().unwrap().extend(scope_metrics);
     READER.force_collect(); // on-demand collect to control when data is flushed
 }

Either way, users will need a supported way to create these types from external data, which means exposing some form of construction contract. We could introduce a parallel set of public DTO types and map them to the ResourceMetrics tree internally, but given that these types are already partially public (getters, Default, trait signatures), creating a separate anticorruption layer won't help without breaking the contract and closing this types.

The same reasoning applies to the PushMetricExporter. It's public but currently unusable since its argument can't be constructed. If the intent is for it not to be called directly, it may be worth reconsidering whether it should remain public in a separate crate.